Skip to content

Add support for post_fields without buffer copy - #332

Open
dovreshef wants to merge 1 commit into
alexcrichton:mainfrom
dovreshef:master
Open

dovreshef wants to merge 1 commit into
alexcrichton:mainfrom
dovreshef:master

Conversation

@dovreshef

Copy link
Copy Markdown

Hi

I wanted to add support for uploading data without the buffer copy between curl and rust, and without copying it in parts into a read function.

Basically support CURLOPT_POSTFIELDS, but unlike today, without setting CURLOPT_COPYPOSTFIELDS.

I've added both the option to support uploading 'static data, which I think is simple.

I'm not so sure about my approach to solving it for Transfer. I'm open to any other approach really, if you have any pointers. This was for me high magic, and I hope I've got it right.

And, of course, all that is in case that is something that you're interested in adding support for.

@alexcrichton

Copy link
Copy Markdown
Owner

I think the reason I didn't originally bind this was the lifetime issue with data. I think we'll probably want to either make this unsafe or figure out a safe operation, because having &'static isn't too too helpful I think?

@dovreshef

Copy link
Copy Markdown
Author

I think the reason I didn't originally bind this was the lifetime issue with data. I think we'll probably want to either make this unsafe or figure out a safe operation, because having &'static isn't too too helpful I think?

You're right. I mostly added it out of completion (to have all the options) and being useful for tests I guess.

What about tying it the lifetime in some way into a transfer object? Does it have to be unsafe?

I'm not sure that what I did is safe, but if not, surely there is some way to make sure that the upload buffer is set to null when the transfer goes out of scope?

@alexcrichton

Copy link
Copy Markdown
Owner

I'm not very familiar with the safety here or this option, and I don't have a ton of time to dig in right now unfortunately. It might be possible though to tie it to the Transfer object.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants